Solution#1813
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your code has a critical bug in the century calculation that causes the E2E test to fail. According to the task requirements, century must be calculated using the year of death (Math.ceil(person.died / 100)), but your code uses the birth year (Math.ceil(p.born / 100)). This produces incorrect century values for people born in one century but died in another. For example, someone born 1832 and died 1905 should have century 20, not 19. The fix is simple: change p.born to p.died in the century calculation on the line that computes Math.ceil(p.born / 100).
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| <td>${p.born}</td> | ||
| <td>${p.died}</td> | ||
| <td>${p.died - p.born}</td> | ||
| <td>${Math.ceil(p.born / 100)}</td> |
There was a problem hiding this comment.
The century calculation should use the year of death (p.died) instead of birth year (p.born). According to the requirements: Math.ceil(person.died / 100). Using born year produces incorrect century values for people born in one century but died in another (e.g., someone born 1832, died 1905 should be century 20, not 19).
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation has a critical blocker that causes the Cypress test to fail. The table generation code is missing the gender column - the test expects to find 'Male' or 'Female' values in the table but the column is not being created. While your previous review mentioned the century calculation fix was successful, the test at cypress/integration/taskGenerateTableDom.spec.js (line 23) confirms the gender field is being filtered out or never added. You need to include gender (or sex depending on your data source) in your table generation logic alongside the other 5 fields (name, born, died, age, century). Once this column is properly included, all 6 cells per row should be rendered correctly.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved. The implementation correctly generates HTML table rows with all 6 required cells (name, gender, born, died, age, century) for each person. Age and century calculations work as specified, the gender values are properly converted from 'm'/'f' to 'Male'/'Female', and rows are correctly appended to the .dashboard table. Great job following the constraint of only modifying main.js. Keep up the solid work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.